-
Notifications
You must be signed in to change notification settings - Fork 22
Fixes #38958 - Add basic Leapp preupgrade report table to the new job invocation page #165
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Add PropTypes and defaultProps to mock components (Table, Pagination) - Fix React Hooks rules violation by moving hooks before early return - Fix camelcase linting for API properties (template_name, preupgrade_report_entries) - Simplify else-if structure to avoid lonely-if error - Fix consistent-return by explicitly returning undefined in early exit - Fix boolean attribute syntax (isEmbedded) - Remove trailing whitespace for prettier compliance 🤖 Generated with [Claude Code](https://claude.com/claude-code) Assisted-By: Claude <noreply@anthropic.com>
adamlazik1
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haven't looked at the whole code yet but test failures are related and need to be addressed.
webpack/components/PreupgradeReportsTable/PreupgradeReportsTable.scss
Outdated
Show resolved
Hide resolved
131319a to
80017aa
Compare
webpack/components/PreupgradeReportsTable/__tests__/PreupgradeReportsTable.test.js
Outdated
Show resolved
Hide resolved
Fixed multiple test issues: - Changed redux-thunk import from named to default export - Added proper mocks for I18n, Pagination, and Table components - Updated Table mock to handle childrenOutsideTbody and customEmptyState props - Updated snapshots for PreupgradeReportsList tests All 61 tests now passing. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Generated-By: Claude <noreply@anthropic.com>
e42f6c1 to
ba5ec56
Compare
.../components/PreupgradeReportsList/__tests__/__snapshots__/PreupgradeReportsList.test.js.snap
Outdated
Show resolved
Hide resolved
57bf4e2 to
e369a39
Compare
Replace snapshot-based testing with React Testing Library assertions that test actual component behavior: - Verify header columns render correctly - Test entry data (titles, hostnames) appears in the document - Check checkbox interactions call the correct handlers - Test selected state rendering - Verify pagination component is present Generated-By: Claude Code 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
webpack/components/PreupgradeReportsTable/PreupgradeReportsTableConstants.js
Outdated
Show resolved
Hide resolved
- wrappers in columns instead of explicity tbody - no tableindexpage - no childrenOutsideTbody - no customEmptyState
1cf15c8 to
c7c4653
Compare
- use perPage over per_page whereever possible - drop custom bottom pagination & friends
c7c4653 to
2c260dd
Compare
webpack/components/PreupgradeReportsTable/PreupgradeReportsTable.scss
Outdated
Show resolved
Hide resolved
webpack/components/PreupgradeReportsTable/PreupgradeReportsTableConstants.js
Outdated
Show resolved
Hide resolved
webpack/components/PreupgradeReportsTable/PreupgradeReportsTableConstants.js
Outdated
Show resolved
Hide resolved
- drop redefined STATUS constant - drop unused css - move rendering function out of constants
92b3a3b to
6a684d4
Compare
|
Thank you all for the reviews and the changes! I'm back, so I'll take it from here again. 😁 |
|
@MariaAga @adamruzicka I see that you addressed everything, and I think the code is great now. However, I can't access the legacy page right now to compare it against the new changes. |
|
Is the new table missing the summary part? or is it for later? |
|
Afaik it was planned to be added later, same as checkboxes+actions and sorting+searching |
|
@kmalyjur in light of my previous comment, is there anything left to be done here and now or can we |
|
Eh, i should have squashed this. Oh well |


TO-DO: Fix tests
#38951 needs to be merged for this PR
Before:

After:
